ENG-1574: Add dual-read console logs to setting setters by sid597 · Pull Request #914 · DiscourseGraphs/discourse-graph

sid597 · 2026-03-26T17:54:04Z

We now print all settings on init and then can call the window.dgDualReadLog() whenever we want to.

Summary by CodeRabbit

New Features
- Added diagnostic logging capabilities to verify settings consistency and identify potential configuration issues for improved troubleshooting.
Refactor
- Improved internal code organization and modularity to enhance maintainability.

Log legacy and block prop values with match/mismatch status when a setting is changed. Fix broken import in storedRelations.

linear · 2026-03-26T17:54:08Z

ENG-1574 Add console logs to existing function check

supabase · 2026-03-26T17:54:11Z

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Log all legacy vs block prop settings on init. Remove setter logging. Expose dgDualReadLog() on window for on-demand use.

JSON.stringify is key-order dependent, causing false mismatches when legacy and block props return keys in different order.

sid597 · 2026-03-27T12:03:21Z

@coderabbitai full review

coderabbitai · 2026-03-27T12:03:26Z

✅ Actions performed

Full review triggered.

coderabbitai · 2026-03-27T12:08:23Z

📝 Walkthrough

Walkthrough

Exports deepEqual utility function from accessors module and enhances settings initialization with optional dual-read consistency comparison between legacy and new settings stores, logging mismatches via console and window-exposed logger when the new store is enabled.

Changes

Cohort / File(s)	Summary
Settings utilities refactor `apps/roam/src/components/settings/utils/accessors.ts`, `apps/roam/src/components/settings/utils/init.ts`	Exports `deepEqual` function and expands `initSchema` to perform optional dual-read consistency checking. Adds `logDualReadComparison` function that reads legacy (feature flags, global, personal, per-node) and new store settings, compares them using `deepEqual`, aggregates mismatches, logs summaries/payloads to console, and exposes logger on `window.dgDualReadLog`. Wrapped in try/catch to warn on failures.

Sequence Diagram

sequenceDiagram
    participant initSchema as initSchema Function
    participant legacyReader as Legacy Settings Reader
    participant newReader as New Store Reader
    participant deepEqual as deepEqual Comparator
    participant logger as logDualReadComparison
    participant console as Console/Window

    initSchema->>initSchema: Check if new settings store enabled
    alt New store enabled
        initSchema->>legacyReader: Read legacy feature flags, global, personal, per-node settings
        legacyReader-->>initSchema: Legacy settings payload
        initSchema->>newReader: Read corresponding block-props/new-store values
        newReader-->>initSchema: New store payload
        initSchema->>logger: Call logDualReadComparison
        logger->>deepEqual: Compare legacy vs new (exclude flag)
        deepEqual-->>logger: Mismatch results
        logger->>logger: Aggregate mismatches by type
        logger->>console: Log summary and full payloads
        console-->>logger: Logged
        initSchema->>console: Expose logDualReadComparison as window.dgDualReadLog
    else New store disabled
        initSchema->>initSchema: Skip dual-read comparison
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

ENG-1465: Add "Use new settings store" feature flag #811: Adds the "Use new settings store" flag and corresponding accessor that the main PR's dual-read logic checks and excludes from comparison.
ENG-1440: Port page groups settings in suggestive mode #793: Introduces the initSchema function that the main PR expands with dual-read consistency comparison logic.
ENG-1519: Add legacy-to-blockprops migration, remove backedBy from schemea #876: Modifies the same settings utils files, adding complementary legacy-read utilities and dual-read/migration comparison logic using the exported deepEqual helper.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'ENG-1574: Add dual-read console logs to setting setters' directly and specifically describes the main change in the pull request—adding dual-read logging functionality via console logs to setting operations, which aligns with the modifications to both accessors.ts and init.ts.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

apps/roam/src/components/settings/utils/init.ts (1)

344-349: Avoid duplicating the feature-flag string literal.

Line 348 hardcodes "Use new settings store", while the same key is also hardcoded in apps/roam/src/components/settings/utils/accessors.ts. Centralizing this key as a shared constant will prevent drift if the flag name changes.

♻️ Suggested refactor

+// e.g. from zodSchema or a shared settings-keys module
+import { USE_NEW_SETTINGS_STORE_FLAG } from "./zodSchema";

   const omitStoreFlag = (
     flags: Record<string, unknown>,
   ): Record<string, unknown> =>
     Object.fromEntries(
-      Object.entries(flags).filter(([k]) => k !== "Use new settings store"),
+      Object.entries(flags).filter(([k]) => k !== USE_NEW_SETTINGS_STORE_FLAG),
     );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/roam/src/components/settings/utils/init.ts` around lines 344 - 349, The
hardcoded feature flag string "Use new settings store" is duplicated; introduce
a single exported constant (e.g., NEW_SETTINGS_STORE_FLAG) in a shared module
and replace the literal in the omitStoreFlag function and the matching reference
in accessors.ts with that constant (update imports accordingly); ensure the
constant name is used in Object.entries(...).filter(([k]) => k !==
NEW_SETTINGS_STORE_FLAG) inside omitStoreFlag and in any accessor functions that
previously referenced the literal.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/roam/src/components/settings/utils/init.ts`:
- Around line 344-349: The hardcoded feature flag string "Use new settings
store" is duplicated; introduce a single exported constant (e.g.,
NEW_SETTINGS_STORE_FLAG) in a shared module and replace the literal in the
omitStoreFlag function and the matching reference in accessors.ts with that
constant (update imports accordingly); ensure the constant name is used in
Object.entries(...).filter(([k]) => k !== NEW_SETTINGS_STORE_FLAG) inside
omitStoreFlag and in any accessor functions that previously referenced the
literal.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f9358337-3c1f-4b25-9aeb-92b543c8f329

📥 Commits

Reviewing files that changed from the base of the PR and between 8830ade and 2397446.

📒 Files selected for processing (2)

apps/roam/src/components/settings/utils/accessors.ts
apps/roam/src/components/settings/utils/init.ts

ENG-1574: Add dual-read console logs to setting setters

c6c4252

Log legacy and block prop values with match/mismatch status when a setting is changed. Fix broken import in storedRelations.

devin-ai-integration bot reviewed Mar 26, 2026

View reviewed changes

Merge origin/eng-1470-test-dual-read-with-flag-on-and-fix-gapsv2

9ad41cf

This comment was marked as resolved.

Sign in to view

ENG-1574: Add init-time dual-read log and window.dgDualReadLog()

699e475

Log all legacy vs block prop settings on init. Remove setter logging. Expose dgDualReadLog() on window for on-demand use.

This comment was marked as resolved.

Sign in to view

sid597 added 2 commits March 27, 2026 17:16

ENG-1574: Fix eslint naming-convention warnings in init.ts

b876cc9

ENG-1574: Use deepEqual instead of JSON.stringify for comparison

1567b3d

JSON.stringify is key-order dependent, causing false mismatches when legacy and block props return keys in different order.

This comment was marked as resolved.

Sign in to view

sid597 added 2 commits March 27, 2026 17:26

ENG-1574: Remove dead code, use deepEqual for comparison

e807ddf

ENG-1574: Fix review feedback — try-catch, flag exclusion, type guard

2397446

coderabbitai bot reviewed Mar 27, 2026

View reviewed changes

sid597 requested a review from mdroidian March 27, 2026 14:51

mdroidian approved these changes Mar 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENG-1574: Add dual-read console logs to setting setters#914

ENG-1574: Add dual-read console logs to setting setters#914
sid597 wants to merge 7 commits intoeng-1470-test-dual-read-with-flag-on-and-fix-gapsv2from
eng-1574-add-console-logs-to-existing-function-check

sid597 commented Mar 26, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

linear bot commented Mar 26, 2026

Uh oh!

supabase bot commented Mar 26, 2026

Uh oh!

devin-ai-integration bot left a comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

sid597 commented Mar 27, 2026

Uh oh!

coderabbitai bot commented Mar 27, 2026

Uh oh!

coderabbitai bot commented Mar 27, 2026

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sid597 commented Mar 26, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

linear bot commented Mar 26, 2026

Uh oh!

supabase bot commented Mar 26, 2026

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

sid597 commented Mar 27, 2026

Uh oh!

coderabbitai bot commented Mar 27, 2026

Uh oh!

coderabbitai bot commented Mar 27, 2026

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sid597 commented Mar 26, 2026 •

edited by coderabbitai bot

Loading